Python: defer turn-scoped after_run providers to the agent loop boundary - #7289
Python: defer turn-scoped after_run providers to the agent loop boundary#7289Yufeng He (he-yufeng) wants to merge 12 commits into
Conversation
Each AgentLoopMiddleware iteration is a full agent run, so CompactionProvider.after_run fired per iteration and rewrote persisted history mid-task (microsoft#7236). Providers can now opt into turn scope with after_run_once_per_turn; iterations defer them via a contextvar, and the loop fires them once at the boundary. CompactionProvider opts in; HistoryProvider keeps its incremental per-run persistence.
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
agent-framework-automation
Bot
added
the
python
There was a problem hiding this comment.
Pull request overview
This PR fixes a harness-loop correctness issue where ContextProvider.after_run was firing once per AgentLoopMiddleware iteration (i.e., mid-turn), by allowing providers to opt into turn-scoped after_run semantics and deferring those providers until the loop boundary. This is intended to prevent providers like CompactionProvider from rewriting persisted history while the loop is still actively using it.
Changes:
- Adds
ContextProvider.after_run_once_per_turn(defaultFalse) to mark providers whoseafter_runshould be deferred to the end of anAgentLoopMiddlewareloop. - Introduces loop-iteration tracking via a contextvar and updates
_run_after_providersto skip turn-scoped providers during loop iterations, then fire them once at loop exit. - Marks
CompactionProvideras turn-scoped and adds tests validating once-per-turn vs once-per-run behavior for both streaming and non-streaming runs.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| python/packages/core/tests/core/test_harness_loop.py | Adds tests asserting turn-scoped providers fire once per loop (streaming/non-streaming) and CompactionProvider opts in. |
| python/packages/core/agent_framework/_sessions.py | Extends ContextProvider with the after_run_once_per_turn opt-in flag and documentation. |
| python/packages/core/agent_framework/_harness/_loop.py | Sets a loop-iteration contextvar around each iteration and fires deferred turn-scoped after_run providers once at loop exit. |
| python/packages/core/agent_framework/_compaction.py | Opts CompactionProvider into once-per-turn after_run semantics. |
| python/packages/core/agent_framework/_agents.py | Adds the loop-iteration contextvar and updates _run_after_providers to implement skip/run logic for turn-scoped providers. |
…s through Two review follow-ups: the contextvar now carries the agent instance so a nested agent.run() inside a loop iteration is not suppressed as if it were an iteration, and the boundary SessionContext forwards the original run options to turn-scoped providers.
GitHub Actions
Inactive
GitHub Actions
Inactive
|
Yufeng He (@he-yufeng) I don't see commit 1ca036d showing up here. |
…xtvar The contextvar marker leaked in two ways. Held across a streamed yield it bled into the caller's context, suppressing turn-scoped providers on an unrelated same-agent run while the stream was paused, and a reset from a different consuming task raised on the token. Keyed to the agent instance, it also swallowed the boundary flush of a nested loop on the same agent with its own session. Stamp the runs the loop drives through their options instead. Run options reach only the inner runs (they never enter the model request), a nested or concurrent run starts with fresh options and keeps its own turn, and there is no token to reset, so stream consumption is safe from any task.
GitHub Actions
Inactive
|
Evan Mattson (@moonbox3) my mistake, the commit landed on the wrong local branch and I pushed that one, so the PR head never had it. It's on the PR now as 6dc2e17 (same change, cherry-picked cleanly, the harness-loop suite passes 96/96 locally). |
GitHub Actions
Inactive
|
Yufeng He (@he-yufeng) Can you address the failing checks here? |
GitHub Actions
Active
GitHub Actions
Active
|
Yufeng He (@he-yufeng) Please address the failing checks |
GitHub Actions
Active
|
Done — merged current main into the branch, so the checks re-run against today's main. The bandit failure was in packages/tools/agent_framework_tools/shell/_docker.py, which this PR doesn't touch and which hasn't changed since July, so it was inherited rather than mine; the fresh run will show whether main has settled it. |
GitHub Actions
Active
|
The pre-commit red was mine after all: bandit flags the _LOOP_ITERATION_TOKEN_KEY string as a hardcoded credential (B105). nosec'd it with the reason inline in 8ec847c. |
GitHub Actions
Active
|
Yufeng He (@he-yufeng) still two failing checks, please address |
|
Please re-open when CI/CD checks are no longer failing. |
|
Evan Mattson (@moonbox3) the two reds are fixed on the branch. Attribution correction from my side: yesterday's nosec addressed bandit, but the failing gate was ruff's S105 plus this repo's no-noqa-comment rule. c4940f9 switches the suppression to the repo's own form (# ruff: ignore[hardcoded-password-string], generated by ruff itself) and the full packages/core ruff check passes locally with --no-fix. I can't reopen this myself (GitHub refused after the push), could you reopen it so CI reruns? |
GitHub Actions
Active
GitHub Actions
Active
GitHub Actions
Active
GitHub Actions
Active
GitHub Actions
Active
|
Yufeng He (@he-yufeng) CI/CD failures are still present. |
GitHub Actions
Active
|
Evan Mattson (@moonbox3) both layers are now addressed on the branch. The ruff gate needed the repo's own suppression form (c4940f9, , nosec kept for the bandit config). Behind it, pyright then flagged two private-usage sites in my own change: the cross-module import of and the assignment on a SessionContext the loop just built. 7620b7f covers both with the repo's existing idiom (the same one uses), with the reason inline. on the touched files is clean locally, and the harness loop suites still pass (105 passed, 6 skipped). |
|
Evan Mattson (@moonbox3) both layers are now addressed on the branch. The ruff gate needed the repo's own suppression form (c4940f9, |
Motivation & Context
With
create_harness_agent(or anyAgentLoopMiddlewareusage) every loop iteration is a full agent run, soCompactionProvider.after_runfires per iteration instead of once per user turn. Fired mid-task, compaction rewrites the persisted history the task still works from (#7236).Description & Review Guide
after_run_once_per_turn(default False onContextProvider). While a loop iteration is active,_run_after_providersskips those providers (tracked through a contextvar the middleware sets around each iteration); when the loop exits, the middleware fires the deferred providers once with the turn-level response viaonly_per_turn=True.CompactionProvideropts in;HistoryProviderkeeps firing per iteration because its persistence is incremental per run._harness/_loop.py(contextvar reset placement and thefinallythat fires the deferred providers) and the skip logic in_run_after_providers.Tests: 4 new tests in test_harness_loop.py (turn-scoped fires once vs run-scoped per iteration, streaming and non-streaming, no-loop baseline, CompactionProvider opt-in). test_harness_loop 92 pass; test_compaction + test_sessions + test_agents 270 pass.
Related Issue
Fixes #7236
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.